Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

feat(MenuItem|AccordionTitle): add ability for removing and customizing the (active|submenu) indicator#721

Merged
mnajdova merged 19 commits intomasterfrom
feat/menu-item-hide-submenu-indicator
Jan 21, 2019
Merged

feat(MenuItem|AccordionTitle): add ability for removing and customizing the (active|submenu) indicator#721
mnajdova merged 19 commits intomasterfrom
feat/menu-item-hide-submenu-indicator

Conversation

@mnajdova
Copy link
Contributor

@mnajdova mnajdova commented Jan 14, 2019

This PR aims to fix #687 (allowing the user to remove the submenu indicator), as well is is adding props for customizing the indicator, by providing an icon shorthand. I have few proposal for the API, and would like to hear your opinion on them.

  1. Add explicit props for all things
  • hideSubmenuIndicator: boolean - indicartes whether the submenu indicators should be shown
  • submenuIndicatorVertical: ShorthandValue - shorthand for icon for the submenu indicators in the vertical menus
  • submenuIndicatorHorizontal: ShorthandValue - shorthand for icon for the submenu indicators in the horizontal menus

Pros: this API provides you to define all indicators in the root menu, and all these will be propagated to all submenus (vertical and horizontal)
Cons: too many properties added for just one slot

  1. Add single property
  • submenuIndicator: boolean | ShorthandValue - this is a single prop that defined whether the submenu indicator shuold be shown (true or false), but also allows the user to specify shorthand for the icon in it.

Props: we are adding just one prop for the submenu indicator, so the user doesn't have to think which one he should apply.
Cons: with this approach, we cannot specify on the root menu what should the submenu indicators in all menus be, as the vertical and horizontal menus are most likely to be different icons. This means, every menu shuold define the submenu indicators in it's first children MenuItems.

3. Add single property and transform the icon - this is currently implemented

  • submenuIndicator: boolean | ShorthandValue - this is a single prop that defined whether the submenu indicator shuold be shown (true or false), but also allows the user to specify shorthand for the icon in it. For the vertical menus, we can transform this icon 90 degrees.

Props: we are adding just one prop for the submenu indicator, so the user doesn't have to think which one he should apply. Allows the user to specify the submenu indicator on the root menu
Cons: not sure whether this would be expected behavior for the client.

TODO:

  • update changelog
  • add Indicator examples

Loading
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ability to remove arrow from submenu

6 participants